[Infra UI] Propagate flyout state to node details page and some fixes #165956

crespocarlos · 2023-09-07T11:14:54Z

Summary

This PR enables state propagation between asset details flyout and full page view.

navigate_to_full_page.mov

There are other places in Kibana that redirect to node details outside the infra plugin such as APM and Observability/Overview. They use the link-to/${assetType}-detail path, so It's best, for now, to keep retro-compatibility with this route and propagate the state via query string.

I've also refactored how we were persisting state via route navigation, to use native the state attribute found in the location object from react-router

How to tests

Start a Kibana instance
Navigate to Infrastructure > Host
- Open the flyout, click in one of the tabs, and click on "Open as Page". The page should open in the same state as the flyout.

apmmachine · 2023-09-07T11:15:20Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…e-flyout-state-to-node-details-page

crespocarlos · 2023-09-08T07:15:55Z

x-pack/plugins/observability_shared/public/hooks/use_link_props.ts

@@ -77,7 +78,7 @@ export const useLinkProps = (
      const navigate = () => {
        if (navigateToApp) {
          const navigationPath = mergedHash ? `#${mergedHash}` : mergedPathname;
-          navigateToApp(app, { path: navigationPath ? navigationPath : undefined });
+          navigateToApp(app, { path: navigationPath ? navigationPath : undefined, state });


Cleaner way of propagating state between routes

crespocarlos · 2023-09-08T07:17:01Z

x-pack/plugins/infra/public/pages/link_to/use_node_details_redirect.ts

+        state: {
+          ...(location.pathname
+            ? ({
+                originAppId: appId,
+                originSearch: location.search,
+                originPathname: location.pathname,
+              } as RouteState)


This won't go in the querystring anymore, creating a shorter and less polluted query param.

elasticmachine · 2023-09-08T09:02:47Z

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

shahzad31

Obs Shared LGTM !!

jennypavlova

Well done! The navigation to the node details works as expected when using absolute dates - I found an issue if you go to the Logs tab and change the dates to relative dates (Last 1 hour for example) then enter a search term the navigation is not correct (it is back to the Overview tab):

logs_and_back_issue.mov

I have a question regarding the Back button - if there are changes inside the node details view to the state should we persist those in case the user goes back to the host flyout?

x-pack/plugins/infra/public/pages/link_to/use_node_details_redirect.ts

x-pack/plugins/infra/public/pages/link_to/redirect_to_node_detail.tsx

crespocarlos · 2023-09-08T11:23:15Z

hi @jennypavlova , thanks for reviewing this.

I will look into the issue you've found.

I have a question regarding the Back button - if there are changes inside the node details view to the state should we persist those in case the user goes back to the host flyout?

I thought about that too. It could, and it's a matter of what the expectations are. I would say that changes made in the node details view shouldn't persist when the user goes back to the flyout. It appears that a new flow starts when you open it as a page. But we can check with @roshan-elastic what he thinks.

A context provider could be a solution for sharing this state between these 2 flows, but I'm not sure if that would work when the flyout starts being used across Kibana as embeddable. I'm afraid this solution will break.

crespocarlos · 2023-09-08T11:40:12Z

@elasticmachine merge upstream

…s-page

jennypavlova

@crespocarlos Thanks for the fix, LGTM 💯
Regarding the state - you are right maybe it's better to not persist the state when coming back to the flyout so it's also easier when the flyout is used in different places.

crespocarlos · 2023-09-08T11:56:19Z

Regarding the state - you are right maybe it's better to not persist the state when coming back to the flyout so it's also easier when the flyout is used in different places.

It's a good use case, to be honest. Let's consider this in future discussions.

kibana-ci · 2023-09-08T12:44:17Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`observabilityShared`	276	277	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`infra`	2.0MB	2.0MB	+1.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`observabilityShared`	43.5KB	43.5KB	+18.0B

Unknown metric groups

API count

id	before	after	diff
`observabilityShared`	277	278	+1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

crespocarlos force-pushed the 164300-propagate-flyout-state-to-node-details-page branch 2 times, most recently from 269b390 to dcc553e Compare September 7, 2023 12:00

Propagate flyout state to node details page and some fixes

0ccb795

crespocarlos force-pushed the 164300-propagate-flyout-state-to-node-details-page branch from dcc553e to 0ccb795 Compare September 7, 2023 12:07

crespocarlos added 2 commits September 7, 2023 14:52

Merge branch 'main' of github.com:elastic/kibana into 164300-propagat…

5c96178

…e-flyout-state-to-node-details-page

Revert changes

66c4980

crespocarlos force-pushed the 164300-propagate-flyout-state-to-node-details-page branch from 2a57869 to e7b38cf Compare September 7, 2023 14:40

Simplifcation

7d2d046

crespocarlos force-pushed the 164300-propagate-flyout-state-to-node-details-page branch from e7b38cf to 7d2d046 Compare September 7, 2023 15:06

Reset flyout url state after closing it

9d4cdf5

crespocarlos commented Sep 8, 2023

View reviewed changes

Simplify state propagation

31bf214

crespocarlos force-pushed the 164300-propagate-flyout-state-to-node-details-page branch from 73e6404 to 31bf214 Compare September 8, 2023 07:22

crespocarlos marked this pull request as ready for review September 8, 2023 09:02

crespocarlos requested review from a team as code owners September 8, 2023 09:02

shahzad31 approved these changes Sep 8, 2023

View reviewed changes

Fix test

311c665

jennypavlova self-requested a review September 8, 2023 09:27

jennypavlova reviewed Sep 8, 2023

View reviewed changes

x-pack/plugins/infra/public/pages/link_to/use_node_details_redirect.ts Show resolved Hide resolved

x-pack/plugins/infra/public/pages/link_to/redirect_to_node_detail.tsx Show resolved Hide resolved

Fix redirect with relative dates

49cf601

crespocarlos requested a review from jennypavlova September 8, 2023 11:38

Merge branch 'main' into 164300-propagate-flyout-state-to-node-detail…

014caf8

…s-page

jennypavlova approved these changes Sep 8, 2023

View reviewed changes

crespocarlos merged commit c6f72c7 into elastic:main Sep 8, 2023

crespocarlos deleted the 164300-propagate-flyout-state-to-node-details-page branch September 8, 2023 13:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Infra UI] Propagate flyout state to node details page and some fixes #165956

[Infra UI] Propagate flyout state to node details page and some fixes #165956

crespocarlos commented Sep 7, 2023 •

edited

Loading

apmmachine commented Sep 7, 2023

crespocarlos Sep 8, 2023

crespocarlos Sep 8, 2023

elasticmachine commented Sep 8, 2023

shahzad31 left a comment

jennypavlova left a comment

crespocarlos commented Sep 8, 2023

crespocarlos commented Sep 8, 2023

jennypavlova left a comment

crespocarlos commented Sep 8, 2023

kibana-ci commented Sep 8, 2023

API count

[Infra UI] Propagate flyout state to node details page and some fixes #165956

[Infra UI] Propagate flyout state to node details page and some fixes #165956

Conversation

crespocarlos commented Sep 7, 2023 • edited Loading

Summary

How to tests

apmmachine commented Sep 7, 2023

🤖 GitHub comments

crespocarlos Sep 8, 2023

Choose a reason for hiding this comment

crespocarlos Sep 8, 2023

Choose a reason for hiding this comment

elasticmachine commented Sep 8, 2023

shahzad31 left a comment

Choose a reason for hiding this comment

jennypavlova left a comment

Choose a reason for hiding this comment

crespocarlos commented Sep 8, 2023

crespocarlos commented Sep 8, 2023

jennypavlova left a comment

Choose a reason for hiding this comment

crespocarlos commented Sep 8, 2023

kibana-ci commented Sep 8, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

crespocarlos commented Sep 7, 2023 •

edited

Loading